[L1T] Extend P2GT members accessors - #51804
Open
artlbv wants to merge 3 commits into
Open
Conversation
…Candidate l1t::P2GTCandidate was missing an easy way to identify Et-sum/Ht-sum type objects, and had no physical (GeV) accessor for isolation and HT (scalarSumPT) -- only the raw hardware ints were exposed, forcing DPGAnalysis/Phase2L1TNanoAOD's NanoAOD table config to hand-multiply by a hardcoded LSB pulled from the python scales config as a workaround. Add isEtSum()/isHtSum() predicates alongside the existing isJet() etc. Add isolationPT()/scalarSumPT() physical-value accessors, filled by L1GTProducer from its live L1GTScales at construction time (the same way pt/eta/phi/z0 already are), so the LSB stays defined in exactly one place (l1tGTScales.py) instead of being duplicated as a constant in DataFormats. operator== is left untouched, still comparing only the bit-exact raw hw fields. Update the NanoAOD table config to use the new accessors instead of the HACK. Adding a persisted double is a schema change, so classes_def.xml gets a new Optional<double> dictionary entry and a ClassVersion bump; the recorded checksum is a placeholder pending a full build.
Unlike the other hw fields, these had no caller: L1GTProducer builds P2GTCandidate via friend access (direct private member assignment), and the only other constructor of this class, L1GTEvaluationInterface.h (a bit-packing board-emulation path with no L1GTScales access), only ever populates the raw hw fields. Keep the class free of dead API.
The placeholder checksum 0 was rejected by edmCheckClassVersion during the DataFormats/L1Trigger dictionary build. Built in CMSSW_20_1_0_pre1 (el9_amd64_gcc13); edmCheckClassVersion reports 3737213310 for the class as extended by this PR, and passes with that value recorded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ZhBBoZkn3gtRtXmW3FYoM
Contributor
|
cms-bot internal usage |
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51804/50794 |
Contributor
|
A new Pull Request was created by @artlbv for master. It involves the following packages:
@BenjaminRS, @battibass, @cmsbuild, @ftorrresd, @quinnanm can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Contributor
|
enable nano |
Contributor
|
test parameters:
|
Contributor
|
@cmsbuild, please test |
Contributor
|
+1 Size: This PR adds an extra 36KB to repository Comparison SummarySummary:
Nano size comparison Summary:
NANO Comparison SummarySummary:
|
Contributor
|
@cms-sw/l1-l2 @cms-sw/xpog-l2 can you take a look? There is some HLT dev that depends on this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR description:
Addresses #51794. FYI @mmusich @BenjaminRS @quinnanm @RobertJWard
l1t::P2GTCandidateexposes only raw hardware ints, so consumers reading the objects straight from the event (e.g. the HLT prototype analyzer, which has no access to the producer'sL1GTScales) cannot tell an Et-sum from an Ht-sum, and cannot get isolation or HT in GeV.DPGAnalysis/Phase2L1TNanoAODworked around this by multiplying the hw value by an LSB hardcoded from the python scales config — the HT line was commented## HACK via hw value!.P2GTCandidate.h: addisEtSum()/isHtSum()predicates andisolationPT()/scalarSumPT()accessors, backed by two newOptional<double>members, throwingstd::invalid_argumentwhen unset like every other getter in the class.L1GTProducer.cc: fill them from the producer's liveL1GTScales, so the LSB stays defined only inl1tGTScales.py. All 7 sites that sethwIsolationPT_(3) orhwScalarSumPT_(4) are covered.classes_def.xml: registerOptional<double>, bumpClassVersion3 → 4 (checksum="3737213310"), keep the v3 record.l1tPh2GTtables_cff.py: use the accessors foriso,relIsoandht.Expected output changes: schema change (v3 files stay readable, new members come back unset); the
l1tGTProducerproduct grows ~9% (~210 B/event); no change to any NanoAOD value, since the producer is configured from the samescale_parameterPSet the hardcoded LSBs were copied from.This is an ABI change. Eight packages
#includethe header and must be rebuilt:DataFormats/L1Trigger,DataFormats/HLTReco,L1Trigger/Phase2L1GT,DPGAnalysis/Phase2L1TNanoAOD,HLTrigger/HLTcore,HLTrigger/HLTfilters,RecoEgamma/EgammaHLTProducers,RecoHGCal/TICL. NB! a partial local checkout instead yields spurious Object doesn't have qualityFlags fromhltTiclSeedingL1 / hltL1TEGammaFilteredCollectionProducer.No dependencies on other PRs or externals. (The relval used below only runs with #51803, which fixes a pre-existing, unrelated breakage of the Phase-2 L1 nano workflows.)
PR validation:
CMSSW_20_1_0_pre1,el9_amd64_gcc13.edmCheckClassVersionpasses standalone — the recorded checksum is the value the tool itself reports.step1.root, single-threaded:L1GTtkPhoton_iso/relIso(n=99),L1GTtkElectron_iso/relIso(n=69) andL1GTscJetSum_ht(n=10) all givemax|PR-BASE| = 0on non-trivial values, as do untouched control branches (hwIso,pt,mht,mhtPhi). NB! The comparison must be single-threaded — the multithreaded Phase-2 L1 chain is not reproducible run-to-run, in the unmodified release as much as here.FEVTDEBUGHLToutput (443 objects, 13l1tGTProducercollections), which exercises the v4 dictionary:isEtSum()/isHtSum()true exactly for the Et-/Ht-sum types and false elsewhere;isolationPT() == hwIsolationPT_toInt()*0.25forCL2Photons/CL2Electrons/GMTTkMuons;scalarSumPT() == hwScalarSumPT_toInt()*0.03125forCL2EtSum/CL2HtSum/GTTPromptHtSum/GTTDisplacedHtSum; both throw where unset.GTTEtSumsets no scalar sum in the producer, so it throws there — unchanged from the existinghwScalarSumPT()behaviour.ClassVersion=3) reads under the v4 dictionary — 198 objects, hardware accessors andobjectType_predicates fine, new members unset, no crash.If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
Not a backport; targets
master. No backport currently intended.